-
Notifications
You must be signed in to change notification settings - Fork 0
회원가입 폼 작업 #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
회원가입 폼 작업 #1
Conversation
20250529 과제 제출합니다 - 회원가입 폼 구현
| <meta charset="UTF-8" /> | ||
| <link rel="icon" type="image/svg+xml" href="/vite.svg" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Vite + React + TS</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분은 별로 중요하진 않은데, 웹사이트 초기화 이후 수정해주시는 습관을 들여주시면 좋을 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사용하지 않는 코드는 제거 필요합니다.
| console.log('SignUp values:', values); | ||
| }; | ||
| return ( | ||
| <Typography variant="h1" component="div"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SignUpForm 컴포넌트가 Typography 컴포넌트에 들어가는 구조는 자연스럽지 못해요.
텍스트를 표현하는 태그이기 때문에 만약 div 태그를 표현하고 싶으셨다면 태그에 대해서 검색 해보시면 좋습니다.
| password?: string; | ||
| username?: string; | ||
| phoneNumber?: string; | ||
| onSubmit?: (values: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onSubmit 타입에 정의 된 values 매개변수의 구조가 복잡해 보여요.
코드 가독성을 위해 아래와 같이 변경하면 좋을 것 같습니다.
type OnSubmitArgs = {
// ... 기존 구조체
}| const [username, setUsername] = useState(initUsername ?? ''); | ||
| const [phoneNumber, setPhoneNumber] = useState(initPhoneNumber ?? ''); | ||
|
|
||
| const handleSubmit = (event: React.FormEvent) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
매개변수에 따로 타입을 지정하시는게 아니라 아래와 같은 방식을 쓰시면 코드 가독성에 도움이 됩니다.
const handleSubmit: FormEventHandler<HTMLFormElement> = (event) => {
event.preventDefault();
onSubmit?.({ id, password, username, phoneNumber });
};| noValidate | ||
| autoComplete="off" | ||
| > | ||
| <div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 div는 현재 역할이 없는 것 같은데 페이지 구조의 복잡도를 줄이기 위해 제거가 필요해 보입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가로, 코드의 통일성을 위해 <Box> 컴포넌트 활용 해주시면 좋을 것 같습니다 :)
| return ( | ||
| <form onSubmit={handleSubmit}> | ||
| <Box | ||
| component="form" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Box> 태그가 현재 component 속성에 의해 form 태그로 렌더링이 되고 있어요.
그런데 바깥에 form 태그가 이미 존재하기 때문에 중첩 form 구조가 되고 있습니다.
둘 중 하나 제거가 필요해 보여요.
개인적인 추천으로는 바깥 form을 남겨두시고 Box form을 제거하시는게 가독성 측면에서 좋아보입니다.
| /> | ||
| </div> | ||
| </Box> | ||
| <Stack spacing={2} direction="row"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stack은 하위에 연속적으로 존재하는 컴포넌트를 배치할 때 사용합니다.
이 경우에는 하위에 Button이 하나밖에 없기 때문에 Box 로 영역을 구분하시는게 직관적일 것 같습니다.
개요
회원가입 폼 작업 했습니다